fix: add authentication to GPU detail, topology, and history endpoints#696
Conversation
Lightheartdevs
left a comment
Lightheartdevs
left a comment
There was a problem hiding this comment.
Audit Review
The fix is correct — these are the only three non-health endpoints in dashboard-api without auth. They expose GPU UUIDs, per-GPU VRAM usage, temperature, power draw, service assignments, and 5-minute utilization history.
The implementation is clean: imports Depends and verify_api_key, adds dependencies=[Depends(verify_api_key)] to the three route decorators. Matches the pattern used everywhere else in the codebase.
Blocking: Frontend doesn't pass auth headers
The dashboard frontend (useGPUDetailed.js) calls these endpoints without an Authorization header:
fetch('/api/gpu/detailed') // No authMerging this PR alone will break the GPU panel on the dashboard with 401s. Needs a coordinated frontend change to pass Authorization: Bearer ${DASHBOARD_API_KEY}.
Either include the frontend fix in this PR, or open a companion frontend PR to merge simultaneously.
|
Thanks for the review! I investigated the concern about The dashboard frontend does use bare In location /api/ {
...
proxy_set_header Authorization "Bearer ${DASHBOARD_API_KEY}";
}The This is the same mechanism that serves every other authenticated endpoint in the codebase — Adding auth to these 3 GPU endpoints closes the last remaining auth gap in dashboard-api, which also protects the directly-exposed port 3002 against unauthorized LAN access (requests that bypass the nginx proxy entirely). No frontend changes needed — the PR is safe to merge as-is. |
All three GPU router endpoints were missing Depends(verify_api_key), allowing unauthenticated access to GPU metrics, topology, and history data. Every other non-health endpoint in the dashboard API requires authentication via Bearer token. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
yasinBursali
force-pushed
the
fix/gpu-endpoints-auth
branch
from
6916408 to
9da34e2
Compare
|
Rebased on latest |
Lightheartdevs
left a comment
Lightheartdevs
left a comment
There was a problem hiding this comment.
Audit: REQUEST CHANGES — will break frontend
The backend fix is correct and necessary — these GPU endpoints expose hardware identifiers and resource utilization data that should require auth. The Depends(verify_api_key) pattern matches every other authenticated endpoint in the codebase.
Blocker: Dashboard frontend sends no Authorization header to these endpoints.
useGPUDetailed.js (and related hooks) call /api/gpu/detailed, /api/gpu/topology, /api/gpu/history without an auth header. Merging this alone will break the GPU dashboard panel with 401 errors.
Fix: Needs a companion frontend PR that adds the Authorization header to these fetch calls. Both should merge together or the frontend PR should merge first.
…tion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addressing review feedbackRe: Frontend doesn't pass auth headers — nginx already handles this. proxy_set_header Authorization "Bearer ${DASHBOARD_API_KEY}";All Change made:
No frontend fetch changes needed — merging the backend auth alone will not cause 401s. |
Lightheartdevs
left a comment
Lightheartdevs
left a comment
There was a problem hiding this comment.
Re-audit: APPROVE — original concern resolved
Verified: nginx.conf line 29 injects Authorization: Bearer ${DASHBOARD_API_KEY} on all /api/ proxied requests. The frontend never needs manual auth headers — nginx handles it transparently. The architecture is sound: API key stays server-side, never exposed to the browser. Adding Depends(verify_api_key) to GPU endpoints is the correct backend hardening.
CI all green.
|
Note: The Rust dashboard-api rewrite (#821) merged and deleted the Python files this PR modifies. This PR modified Please rebase or rewrite against the current |
2 participants
What
Add missing authentication to three GPU API endpoints that were accessible without credentials.
Why
GET /api/gpu/detailed,/api/gpu/topology, and/api/gpu/historyhad noDepends(verify_api_key)— every other non-health endpoint in dashboard-api requires auth. These endpoints expose per-GPU UUIDs, VRAM usage, topology, and utilization history.How
Dependsandverify_api_keyimports togpu.pydependencies=[Depends(verify_api_key)]to all 3 route decoratorsupdates.pyconventionTesting
Review
Critique Guardian: APPROVED (all four pillars clean)
Platform Impact
All platforms